-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avail DA Adapter in Rollkit #6
Conversation
Integrated avail da adapter in rollkit
…avail-da-adapter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my side all looks good, comments are mostly cosmetics. If I'm not wrong, there are few things to consider in the future, or maybe to put comments in the code:
- KeyPair if from seed, I guess that should be more configurable
- Does the rollkit retries if block is not yet available, thats not explicit in adapter code?
- Is there a case when multiple blocks are fetched in one call
- Does
Message
field needs concanated extrinsics
Hope that helps :)
da/avail/avail.go
Outdated
BaseHeader: types.BaseHeader{ | ||
Height: blockNumber, | ||
}, | ||
AggregatorsHash: make([]byte, 32), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we need to send any calculated hash here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anilcse ,Actually celestia block structure and avail block structure are different . I manually assigned the appdata(got from avail endpoints) into txs
part of the celestia block with the help of helper functions. So we are not sure about other fields like AggregatorHash,IntermediateStateRoots etc.For now I'll remove those empty fields or If you have any idea regarding how to get those fields, please suggest.Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@murthyvitwit any idea about these?
return err | ||
} | ||
|
||
keyringPair, err := signature.KeyringPairFromSecret(seed, 42) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is a secure way of doing it. Did you check other cosmos implementations on how this is being handled?
return err | ||
} | ||
|
||
key, err := types.CreateStorageKey(meta, "System", "Account", keyringPair.PublicKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to create storate key everytime we are trying to submit the data?
No description provided.